compiler: Suppress incorrect useless_building warning - #11477
Conversation
CT Test Results 2 files 335 suites 8m 41s ⏱️ Results for commit e39e930. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts
// Erlang/OTP Github Action Bot |
bjorng
left a comment
There was a problem hiding this comment.
Please move the test case to warnings_SUITE in the compiler application, because your fix is not actually in erl_lint.
I will have to think a little bit more about the actual fix.
| Sub; | ||
| descend(Core, #sub{top=true}=Sub) -> | ||
| case should_suppress_warning(Core) of | ||
| case should_suppress_warning(Core) andalso not is_result_unwanted(Core) of |
There was a problem hiding this comment.
By substituting the definition of should_suppress_warning/1 into this expression:
should_suppress_warning(Core) andalso not is_result_unwanted(Core)
we will get:
(is_compiler_generated(Core) orelse is_result_unwanted(Core)) andalso not is_result_unwanted(Core)
which can in turn be simplified to:
is_compiler_generated(Core) andalso not is_result_unwanted(Core)
This says that we remain at the top as long as Core is compiler-generated and the result is wanted. The last part doesn't make much sense. It seems that if Core is compiler-generated we should always stay at the top, regardless of whether the expression is wanted or not. Therefore, I think that the condition should be:
| case should_suppress_warning(Core) andalso not is_result_unwanted(Core) of | |
| case is_compiler_generated(Core) of |
With this change, your new test case and all the existing test cases in warnings_SUITE still pass.
64edbc0 to
e39e930
Compare
Fix #11472